Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Group module genesis in/export #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Group module genesis in/export #60

wants to merge 1 commit into from

Conversation

alpe
Copy link
Collaborator

@alpe alpe commented Mar 20, 2020

After #59

See #46

Example json export

{
  "Params": {
    "maxCommentLength": 255
  },
  "GroupSeq": "1",
  "Groups": [
    {
      "key": "AAAAAAAAAAE=",
      "value": {
        "group": "1",
        "admin": "cosmos1q7msxfk6682hgqyhfcywg9tchm86mrhwguyxaj",
        "comment": "ɠĞ辡烴Oʓʭi]",
        "version": "1",
        "totalWeight": "10236414746480987465.000000000000000000"
      }
    }
  ],
  "GroupMembers": [
    {
      "key": "AAAAAAAAAAEQ1bG7ECisk9glZHBZK1r7iZhXwQ==",
      "value": {
        "group": "1",
        "member": "cosmos1zr2mrwcs9zkf8kp9v3c9j266lwyes47pfprx2k",
        "weight": "1351741829321525969.000000000000000000",
        "comment": "擜ħ璝ʣ鏗烍Œ铟ǽv孬ƅ毬¢"
      }
    },
    {
      "key": "AAAAAAAAAAGCs/HJsMn9gt63fxzQiwCGltYv+g==",
      "value": {
        "group": "1",
        "member": "cosmos1s2elrjdse87c9h4h0uwdpzcqs6tdvtl65ca096",
        "weight": "8884672917159461496.000000000000000000",
        "comment": "騹厛Èñ嘦譳AO鳬闿|儳Da"
      }
    }
  ],
  "GroupAccountSeq": "1",
  "GroupAccounts": [
    {
      "key": "NLDsj/teZskpLG6+SCyBk8lFYz0=",
      "value": {
        "base": {
          "groupAccount": "cosmos1xjcwerlmtenvj2fvd6lystypj0y52ceassan0f",
          "group": "1",
          "admin": "cosmos1q7msxfk6682hgqyhfcywg9tchm86mrhwguyxaj",
          "comment": "ɠĞ辡烴Oʓʭi]",
          "version": "1"
        },
        "decisionPolicy": {
          "threshold": {
            "threshold": "10236414746480987464.000000000000000000",
            "timout": "999s"
          }
        }
      }
    }
  ],
  "ProposalSeq": "100",
  "Proposals": [
    {
      "key": "AAAAAAAAAAE=",
      "value": {
        "base": {
          "groupAccount": "cosmos1xjcwerlmtenvj2fvd6lystypj0y52ceassan0f",
          "comment": "峷苫尠$MoM8ǔ譢üIJß",
          "proposers": [
            "cosmos1s2elrjdse87c9h4h0uwdpzcqs6tdvtl65ca096"
          ],
          "submittedAt": "2265-09-02T07:31:08.212292270Z",
          "groupVersion": "1",
          "groupAccountVersion": "1",
          "status": "PROPOSAL_STATUS_CLOSED",
          "result": "PROPOSAL_RESULT_REJECTED",
          "voteState": {
            "yesCount": "0.000000000000000000",
            "noCount": "0.000000000000000000",
            "abstainCount": "8884672917159461496.000000000000000000",
            "vetoCount": "0.000000000000000000"
          },
          "timeout": "2265-09-02T07:47:47.212292270Z",
          "executorResult": "PROPOSAL_EXECUTOR_RESULT_NOT_RUN"
        }
      }
    }
  ],
  "Votes": [
    {
      "key": "AAAAAAAAAAGCs/HJsMn9gt63fxzQiwCGltYv+g==",
      "value": {
        "proposal": "1",
        "voter": "cosmos1s2elrjdse87c9h4h0uwdpzcqs6tdvtl65ca096",
        "choice": "ABSTAIN",
        "comment": "峷苫尠$MoM8ǔ譢üIJß",
        "submittedAt": "2265-09-02T07:31:08.212292270Z"
      }
    },
    {
      "key": "AAAAAAAAAAKCs/HJsMn9gt63fxzQiwCGltYv+g==",
      "value": {
        "proposal": "2",
        "voter": "cosmos1s2elrjdse87c9h4h0uwdpzcqs6tdvtl65ca096",
        "choice": "YES",
        "comment": "AŨȖy俈ɒɶǙP",
        "submittedAt": "2680-09-17T07:53:39.661177005Z"
      }
    }
  ]
}

@alpe alpe requested a review from aaronc March 23, 2020 13:30
@alpe alpe marked this pull request as ready for review March 23, 2020 13:30
@alpe alpe force-pushed the group_app_module branch 2 times, most recently from 76a6980 to 4c156db Compare April 2, 2020 09:20
Copy link
Member

@aaronc aaronc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it probably makes sense to do this in coordination with cosmos/cosmos-sdk#5917 as we will likely standardize on an approach for genesis in an ADR. I think the approach of using tables to do the exporting is sensible.

voteByProposalBaseIndex orm.UInt64Index
voteByVoterIndex orm.Index
VoteByProposalBaseIndex orm.UInt64Index
VoteByVoterIndex orm.Index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for making all of these public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed them available for tests. Indexes are read only and this should not create any harm.

@alpe alpe changed the base branch from orm_validation_redesign to master April 15, 2020 13:50
Comment on lines 314 to 326
message GenesisState {
option (gogoproto.equal) = true;
option (gogoproto.goproto_stringer) = false;
Params Params = 1 [(gogoproto.nullable)=false];
Params Params = 1 [(gogoproto.nullable) = false];
uint64 GroupSeq = 2;
bytes Groups = 3 [(gogoproto.casttype) = "encoding/json.RawMessage"];
bytes GroupMembers = 4 [(gogoproto.casttype) = "encoding/json.RawMessage"];
uint64 GroupAccountSeq = 5;
bytes GroupAccounts = 6 [(gogoproto.casttype) = "encoding/json.RawMessage"];
uint64 ProposalSeq = 7;
bytes Proposals = 8 [(gogoproto.casttype) = "encoding/json.RawMessage"];
bytes Votes = 9 [(gogoproto.casttype) = "encoding/json.RawMessage"];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what if we were to take an approach like this where AnyGroupAccountMetadata and AnyProposal come from #63 and leverage google.protobuf.Any?

message GenesisState {
     Params params = 1;
     uint64 group_seq = 2;
     repeated GroupMetadata groups = 3;
     repeated GroupMember group_members = 4;
     uint64 group_account_seq = 5;
     AnyGroupAccountMetadata group_acconts = 6;
     uint64 proposal_seq = 7;
     repeated AnyProposal proposals = 8;
     repeated Vote votes = 9;
 }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With using concrete types rather than json bytes we can not simply let the orm layer do the en-/decoding. I am wondering now what are the benefits of concrete types here? The genesis is only used for data import/ export.
We won't get additional type safety as this is already covered within orm. But I guess it is easier to read and understand what would be inside the json bytes. Although with Any types this is not obvious again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you have any more thoughts after seeing the Any proposal? Also are you familiar with how the current SDK genesis encodes interfaces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be a bit biased by the implementation already. The orm store requires concrete types and won't work with interfaces. For the genesis in/export I was just focused on "it should work automated by the orm framework layer". I can spike a protobuf version

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants